Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce archivedb key lengths by 1 byte #2113

Merged
merged 10 commits into from
Sep 28, 2023

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

Currently we prefix all user database keys with a 0x00 byte to provide a namespace for metadata. However, we can maintain the invariants of database keys without including that additional byte per key.

How this works

User database keys are defined as: keyLen | key | height
Metadata database keys as: kenLen+1 | key

In order for the user db keys to start with the same beginning as the metadata db keys, the user db keys must be one byte longer. As an example:

newDBKeyFromUser("hello")    -> 5 || hello
newDBKeyFromMetadata("hell") -> 5 || hell

It's clear to see that a user db key (prefix!) can never itself be equal to or a prefix of a metadata db key.

How this was tested

Existing tests + added a new fuzz test. fwiw the fuzz test is pretty questionably useful... because of how few branches there are in the tested code.

@StephenButtolph StephenButtolph added this to the v1.10.12 milestone Sep 28, 2023
@StephenButtolph StephenButtolph added storage This involves storage primitives cleanup Code quality improvement labels Sep 28, 2023
@StephenButtolph StephenButtolph self-assigned this Sep 28, 2023
x/archivedb/key.go Outdated Show resolved Hide resolved
keyLen := len(key)
dbKeyMaxSize := binary.MaxVarintLen64 + keyLen
dbKey := make([]byte, dbKeyMaxSize)
offset := binary.PutUvarint(dbKey, uint64(keyLen)+1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the +1 here making the prefix different than the user keys

x/archivedb/key.go Outdated Show resolved Hide resolved
x/archivedb/key.go Outdated Show resolved Hide resolved
@StephenButtolph StephenButtolph merged commit b4f5845 into dev Sep 28, 2023
13 of 14 checks passed
@StephenButtolph StephenButtolph deleted the reduce-archivedb-key-lengths branch September 28, 2023 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement storage This involves storage primitives
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants